bubblewrap: Add --not-a-security-boundary flag to enable fail-open behavior#751
bubblewrap: Add --not-a-security-boundary flag to enable fail-open behavior#751ao2 wants to merge 1 commit into
--not-a-security-boundary flag to enable fail-open behavior#751Conversation
--not-a-security-boundary flag to enable fail-open …--not-a-security-boundary flag to enable fail-open behavior
|
cc @smcv even though it's still a draft |
…behavior Some callers of bwrap (e.g. xdg-dbus-proxy, Steam Runtime) use it purely to adjust filesystem layout, without any expectation of a security boundary between the sandbox and the host. For these callers, hard failures during sandbox setup (such as an automount timeout on a bind source) are unnecessarily fatal. So add a new `--not-a-security-boundary` option that can be used to relax the bubblewrap behavior in these specific cases, and allow it to "fail-open". In the first implementation enable the "fail open" behavior only to the case where bwrap fails to remount a submount with different flags during a `--bind` or `--bind-fd` operations, and still "fail close" for operations like `--dev-bind` and `--ro-bind` which are supposedly more critical. Signed-off-by: Antonio Ospite <antonio.ospite@collabora.com>
e64137e to
418a4f1
Compare
smcv
left a comment
There was a problem hiding this comment.
I'd like some smoke-test coverage for this in the test suite. Setting up a failing automounter (so that there will genuinely be an error to ignore) is probably too difficult to do in bubblewrap's test suite, but we can at least exercise the happy path where the new option makes no practical difference.
| (op->type == SETUP_DEV_BIND_MOUNT ? BIND_DEVICES : 0) | | ||
| ((op->type == SETUP_BIND_MOUNT && | ||
| opt_not_a_security_boundary) ? BIND_FAIL_OPEN : 0), | ||
| source, dest); |
There was a problem hiding this comment.
Why does only --bind fail open? If bubblewrap is not a security boundary, shouldn't --dev-bind and --ro-bind be equally willing to fail open?
Separately, I think we're getting enough flags that the repeated ?: operators make it harder to read, and it would be better more like:
bind_option_t bind_flags = 0;
if (opt_not_a_security_boundary)
bind_flags |= BIND_FAIL_OPEN;
if (op->type == SETUP_RO_BIND_MOUNT)
bind_flags |= BIND_READONLY;
if (op->type == SETUP_DEV_BIND_MOUNT)
bind_flags |= BIND_DEVICES;
/* etc. */
setup_op_bind_mount (bind_flags, source, dest);|
The commit message should have a
Optionally also a mention of |
|
For manual testing, I see you've described how to reproduce the problem in ValveSoftware/steam-for-linux#10571 (comment) (it might be useful to summarize that in the commit message). It would be useful if you could create other mount points - perhaps What I expect will happen is that Similarly, if you request mounting them read-only, something like But it would be good to confirm this. |
| host system. When this option is given, certain non-fatal sandbox | ||
| setup failures (such as a bind mount failing because an automounter | ||
| did not respond in time) will produce a warning and will be skipped, | ||
| rather than causing <command>bwrap</command> to exit with an error. |
There was a problem hiding this comment.
Perhaps worthwhile to say something like
| rather than causing <command>bwrap</command> to exit with an error. | |
| rather than causing <command>bwrap</command> to exit with an error. | |
| The effect of this option might be extended to make other sandbox | |
| setup operations non-fatal in future releases of bubblewrap. |
to give us space to make more operations fail open if we find that it's desirable.
Some callers of bwrap (e.g. xdg-dbus-proxy, Steam Runtime) use it purely to adjust filesystem layout, without any expectation of a security boundary between the sandbox and the host.
For these callers, hard failures during sandbox setup (such as an automount timeout on a bind source) are unnecessarily fatal.
So add a new
--not-a-security-boundaryoption that can be used to relax the bubblewrap behavior in these specific cases, and allow it to "fail-open".